Skip to content

fix: refuse to run as root unless --allow-root is given - #116

Open
fismif wants to merge 3 commits into
eclipse-enclave:mainfrom
fismif:fix/root-guard
Open

fismif wants to merge 3 commits into
eclipse-enclave:mainfrom
fismif:fix/root-guard

Conversation

@fismif

@fismif fismif commented Sep 28, 2026 •

Copy link
Copy Markdown

What it does

Part of #106 (1 of 2); the sensitive-mount guard follows in a separate PR.

Root guard. Running enclave as root, directly or through sudo, leaves root-owned files in the project and, under sudo -E, in the regular user's config, state, and cache roots, and later runs as the regular user fail on them. Under rootful Docker the agent is also host root on every bind mount. Enclave now refuses to run as root:

  • The check (internal/app/root_guard.go) runs early in app.Run, before anything writes state. Help, version, and shell completion still work.
  • --allow-root or ENCLAVE_ALLOW_ROOT=1 opts in, and each allowed run prints a warning. There is deliberately no config key, so neither global nor project config can grant it.
  • The refusal is a single line, so for tools|features add|update|remove --json it lands in the result envelope's error field. It names the sudo user when there is one and points to the docker group and rootless podman.
  • Host user commands pass the opt-in on to scripts that re-invoke $ENCLAVE_BIN.
  • The RPM smoke test runs as root in its job container, so it now asserts the refusal and then runs with ENCLAVE_ALLOW_ROOT=1.

Root sessions work. Getting past the guard was not enough: no image could be built for UID 0 (a root host, or --build-uid 0).

  • Dockerfile: the user setup renamed the existing user with the build UID to agent, and usermod cannot rename root while the build runs as root. A new UID 0 branch adds agent as a second passwd and group name for UID 0 and leaves root alone. The existing branches, which non-root builds take, are unchanged.
  • QEMU bundle (build-bundle.sh): the same approach, because busybox adduser refuses a UID in use.
  • Lookups by UID return root's entry, which comes first. RUN steps therefore got HOME=/root, and the build helpers landed off the agent's PATH (enclave-install-tool: not found). The UID 0 branch replaces /root with a link to /home/agent.
  • Sessions got USER=root and HOME=/root the same way, so the default git identity was root@enclave. The runtime now exports USER=agent and HOME=/home/agent for UID 0 images, as the QEMU backend already did. Admin sessions keep root's values, and HOME/USER from the project .env still win.
  • effectiveBuildIdentity moves to model.EffectiveBuildIdentity, so the image build and the runtime share the "--build-uid, else the host UID" rule.

Worth a close look: in UID 0 images /root becomes a symlink to /home/agent. Root's passwd entry is untouched. On the Debian and Ubuntu bases /root only holds .bashrc and .profile, which the agent's home gets from skel; a custom base image with more in /root would lose it. Non-root images keep a real /root.

Docs: a new "Running as root" section in docs/cli-reference.md, plus configuration.md, security/README.md, windows.md, README.md, ARCHITECTURE.md, and DEV.md.

How to test

make test covers the guard (internal/app/root_guard_test.go), flag parsing (internal/cli/parse_test.go), and the UID 0 session environment (internal/runtime/runtime_devcontainer_test.go).

As root, from a throwaway directory (the UID 0 agent can write root-owned files into the project):

sudo ./bin/enclave ps              # refused on one line, exit 1
sudo ./bin/enclave --version       # works
sudo ./bin/enclave --allow-root --tool claude --backend docker \
  --image-name enclave-uid0-test:latest shell -- -lc 'id -u; echo "$USER $HOME"'
                                   # 0, then: agent /home/agent
sudo ./bin/enclave --allow-root --tool claude --backend qemu \
  shell -- -lc 'id -u'             # 0 (needs KVM)

Without sudo, --build-uid 0 --build-gid 0 takes the same build path. Point the XDG roots at a scratch directory, because the UID 0 agent leaves root-owned files behind:

E=$PWD/bin/enclave S=$(mktemp -d); mkdir -p "$S/project"; cd "$S/project"
export XDG_CONFIG_HOME=$S/config XDG_STATE_HOME=$S/state XDG_CACHE_HOME=$S/cache
"$E" --tool claude --backend docker --build-uid 0 --build-gid 0 \
  --image-name enclave-uid0-test:latest shell -- -lc 'id -u; echo "$USER $HOME"'
                                   # 0, then: agent /home/agent
# cleanup: the files are root-owned, so remove them through a container
docker run --rm --user 0 --entrypoint rm -v "$S:/s" enclave-uid0-test:latest \
  -rf /s/project /s/config /s/state /s/cache
docker rmi enclave-uid0-test:latest

Leave out --slim for now (see Follow-ups).

What I verified:

  • make build, make lint, and make test. The only failure is in internal/wslshim, which fails on any host with /usr/bin/enclave installed (see Follow-ups).
  • As root: Docker and QEMU sessions run with the agent as UID 0. Without the opt-in the run is refused, and nothing is written.
  • As a regular user with --build-uid 0: USER=agent, HOME=/home/agent, default git identity agent@enclave, a writable home, and working default features. The admin shell keeps USER=root.
  • Non-root sessions are unchanged: the host UID, named agent, with a real /root.
  • The Dockerfile user setup in isolation for UID/GID 0/0, 0/1000, 1000/1000, and 00/00, the bundle user setup, and a full UID 0 QEMU bundle build.

Follow-ups

Related to this change:

  • --allow-root=false still opts in, because bool flags ignore their value (as --verbose does). That needs a general parser fix, left out of this PR.
  • Shell completion runs before the guard, and its completers resolve paths, which can extract the embedded assets into the cache. Completing as root can therefore leave root-owned cache files.
  • Git refuses the project ("detected dubious ownership") when the agent's UID differs from the project owner, for example sudo enclave --allow-root in a regular user's checkout. That is git's own protection, and enclave sets no safe.directory. Should we document it or handle it?
  • Not covered: podman as root, and devcontainer remoteUser: root (left out on purpose). QEMU UID 0 bundles don't get the /root link; their build doesn't need it, but tools that ignore $HOME, such as ssh, use /root in the VM.

Pre-existing, found while testing:

  • --slim builds fail for every UID with "/extensions/features": not found. prepareBuildContext stages only the selected features, but the Dockerfile copies extensions/features unconditionally.
  • The IDE bridge mount (internal/runtime/ide_bridge.go) is nested in the Claude config store, and the container runtime creates its ide mount point there as root on the host. fix: pre-create the tool skills directory in the config store #93 pre-created skills and memory, but not ide.
  • TestProbeScriptExitsNotFoundWhenNothingIsInstalled in internal/wslshim fails on hosts with /usr/bin/enclave installed, which is one of the probe's fallback paths.

Breaking changes

  • This PR introduces breaking changes and has been coordinated with maintainers.

Anyone who runs enclave as root today is now refused until they pass --allow-root or set ENCLAVE_ALLOW_ROOT=1. That includes CI job containers, sudo, and WSL distributions created with wsl --import, which log in as root. The policy (refuse by default, a real opt-in, no config key) was agreed with the project lead. Separately, the Dockerfile change alters the image hash, so every image rebuilds once.

Review checklist

@fismif
fismif requested a review from xai September 28, 2026 10:40

@xai xai left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @fismif for the detailed description and for calling out the remaining limitations. The root-only scope for this PR makes sense, and overall it looks pretty good!

Turns out, Enclave main doesn't build images as uid 0 currently, which I didn't know because I was never tried it until now 😀

At this point, I'd even split out the fix for the currently broken "build as uid 0" behavior into a separate PR.

Having the guard itself is a major UX improvement over the current build errors that a user would receive wehen trying to run it as root. Having bypass mechanisms is also good and their code looks fine to me.

While reviewing this, I also stumbled across a separate problem, (#132), that we also should fix in a dedicated PR.

Comment on lines +121 to +126
# The job container runs as root, which enclave refuses without an opt-in.
if enclave tools >/dev/null 2>&1; then
echo "enclave ran as root without ENCLAVE_ALLOW_ROOT" >&2
exit 1
fi
ENCLAVE_ALLOW_ROOT=1 enclave tools >/dev/null

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This updates the later installation smoke test, but the earlier build-rpm-fedora job also runs as root. Its 'Verify packaged runtime assets' step calls scripts/verify-package-assets.sh, which invokes Enclave twice without an override. CI currently fails at that verification step, so test-fedora is skipped. Please pass --allow-root explicitly to both Enclave invocations in the verifier and use the flag for the allowed invocation here too. The verifier's negative case must get past the guard and still fail because embedded assets are unavailable; a root-refusal error would test the wrong behavior. Please retain the separate assertion here that running without the override is refused.

An out-of scope follow-up issue would be to fix the ci job for fedora. Currently, the fedora ci job runs in a fedora:44 job container and apparently the default user is root there. The follow-up should try setting a non-privileged user in the container.

Comment thread docs/ARCHITECTURE.md Outdated
Comment on lines +357 to +362
For UID 0 (a root host with `--allow-root`, or `--build-uid 0`), the Dockerfile
adds `agent` as a second name for UID 0 instead of renaming `root`, which
`usermod` refuses while the build runs as root; the QEMU bundle build does the
same. Lookups by UID return root's passwd entry, which comes first, so the
Dockerfile also replaces `/root` with a link to `/home/agent` for `RUN` steps,
and the runtime sets `HOME` and `USER` for the agent in sessions.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would defer this to a follow-up PR and only do the guard and the --allow-root bypass in this PR without fixing the UID 0 build issue.

Building with uid 0 is broken on main anyway and triggering the guard and an understandable error message is a much nicer UX than the current behavior. If someone then uses --allow-root, they will still get the current broken state, but we can introduce that fix in a separate PR.

Comment thread docs/cli-reference.md Outdated
Comment on lines +306 to +308
enclave refuses to run as root, including through `sudo`. The agent's container user takes the host UID, so as root the agent runs as UID 0 (named `agent`, a second name for root), which is host root on bind-mounted directories under rootful Docker. Files enclave and the agent write, both in the project and in the stores under the config, state, and cache roots, become root-owned, and later runs as the regular user fail on them (with `sudo -E`, those roots are in the regular user's home). Run enclave as a regular user with access to the Docker socket (see the [requirements](../README.md#requirements)), or use rootless podman with `--backend podman`.

To run as root anyway, for example in a CI job container, pass `--allow-root` or set `ENCLAVE_ALLOW_ROOT=1`; each such run prints a warning; sessions then run with the agent as UID 0, or as the UID given with `--build-uid`. The opt-in has no config key, so neither global nor project config can grant it. Help and version output work without it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment in ARCHITECTURE.md

Comment thread docs/DEV.md Outdated
Comment on lines +302 to +308
`runningAsRoot` seam. A root host builds the image for UID 0: the Dockerfile
and the QEMU bundle build add `agent` as a second name for UID 0, and
`applyUIDZeroAgentEnv` in `internal/runtime` sets `HOME` and `USER` for the
agent, because lookups by UID return root's passwd entry. To test that path as
a regular user, pass `--build-uid 0 --build-gid 0` with `XDG_CONFIG_HOME`,
`XDG_STATE_HOME`, and `XDG_CACHE_HOME` pointing at a scratch directory: the UID 0
agent leaves root-owned files behind.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment in ARCHITECTURE.md

Comment thread internal/app/app.go Outdated
Comment on lines +60 to +65
// The guard runs before anything writes state (the tool question, asset
// extraction, stores), so a refused root run leaves no root-owned files.
// Folding the env opt-in into the options lets validation and per-tool
// re-resolution see one value.
parsed.Options.AllowRoot = rootAllowed(parsed.Options.AllowRoot)
if err := checkRootGuard(parsed.Options.AllowRoot); err != nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard is still reached after writing paths. Run calls discoverUserCommands before parsing, and its ResolveHostHome call creates and deletes a temporary file. More importantly, dynamic completion runs inside cli.Parse: __complete run --tool "" can extract embedded assets, then return before this guard. That leaves a path to root-owned cache files without an opt-in. Please make discovery and the exempt help/version/completion paths read-only, with writing initialization behind the guard. The existing test only checks whether HOME is empty afterward, so it misses temporary writes; please cover completion asset extraction as well. Both behaviors were reproduced with the existing root-check test seam, without running an actual root container.

Comment on lines +93 to +98
if [ "$uid" -eq 0 ]; then
# busybox adduser refuses a UID in use: add the agent as a second name for root.
echo "agent:x:0:$gid::/home/agent:/bin/bash" >> /etc/passwd
echo "agent:!:::::::" >> /etc/shadow
mkdir -p /home/agent
chown "0:$gid" /home/agent

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment in ARCHITECTURE.md

Comment thread Dockerfile Outdated
Comment on lines +91 to +94
RUN if [ "${USER_ID}" -eq 0 ]; then \
# UID 0 (root host with --allow-root, or --build-uid 0): usermod cannot
# rename root while the build runs as root, so add the agent as a
# second name for UID 0 instead.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy for this PR to address only the root-guard part of #106. This alone is a great UX improvement! I would suggest to move the new UID-0 image and runtime support into a separate PR: the Dockerfile and QEMU account changes, the UID-0 HOME/USER handling, and the build-identity extraction needed for that handling.

Those changes introduce separate compatibility questions, including deleting existing /root content and remapping a UID-0 agent. We don't need to solve those here, as the behavior on main is broken anyway in these cases. For this PR, --allow-root or the respective env variable can bypass the new guard without promising to repair previously unsupported root-image workflows; please make that scope clear in the docs. Keep the guard, focused tests/docs, generated flag support, and necessary CI adjustments.

@fismif
fismif force-pushed the fix/root-guard branch 3 times, most recently from c52fe27 to f05e4d0 Compare October 5, 2026 11:36
fismif added 2 commits October 5, 2026 11:42
When run as root, directly or through sudo, enclave and its agent leave
root-owned files in the project, and under sudo -E also in the regular
user's config, state, and cache roots, so later runs as the regular user
fail on them. Under rootful Docker the agent is also host root on every
bind-mounted directory. Enclave now refuses to run as root before it
writes any state. --allow-root or ENCLAVE_ALLOW_ROOT=1 opts in; there is
deliberately no config key, so neither global nor project config can
grant it. Help and version output work without the opt-in, the refusal
stays on one line so it fits the --json result envelope, and user host
commands pass the opt-in on to scripts that re-invoke $ENCLAVE_BIN. The
RPM smoke test runs as root in its job container, so it now checks the
refusal and then runs with ENCLAVE_ALLOW_ROOT=1.

The opt-in has to lead to a working session, but an image for UID 0
could not be built: the Dockerfile renamed the user that already had the
build UID to agent, and usermod cannot rename root while the build runs
as root. The QEMU bundle build failed too, because busybox adduser
refuses a UID in use. Both now add agent as a second passwd name for
UID 0 and leave the root entry alone.

Lookups by UID still return root's entry, which comes first. RUN steps
therefore got HOME=/root, which put the build helpers in
/root/.local/bin, off the agent's PATH, so the UID 0 branch replaces
/root with a link to /home/agent. Sessions got USER=root and HOME=/root
the same way, which made the default git identity root@enclave, so the
runtime now exports USER=agent and HOME=/home/agent for UID 0 images, as
the QEMU backend already did. Admin sessions keep root's values. The
build UID rule moves to model.EffectiveBuildIdentity so the image build
and the runtime share it.

Non-root builds take the same path as before, but the Dockerfile change
alters the image hash, so every image rebuilds once.

Tested as root with Docker and QEMU sessions and the refusal, and as a
regular user with --build-uid 0. Podman as root and devcontainer
remoteUser: root are not covered.

Part of eclipse-enclave#106.
Address the review of the root guard. Help, version, and shell
completion skip the guard, so nothing before it may write: user command
discovery no longer probes HOME for writability, and the completers
never extract the embedded assets. A self-contained binary therefore
completes tool and feature names only once another command has
extracted them. Tests pin directory mtimes under a temporary HOME to
catch writes that are undone again, including completion with the real
embedded assets.

UID 0 image and runtime support moves to a separate change, so the
Dockerfile, the QEMU bundle build, and the runtime are back to main;
--allow-root now only skips the check. The Fedora RPM jobs run as root,
so the package asset check passes --allow-root, and the install smoke
test asserts the refusal before running with the flag.
Read-only completion found no app root until a regular command had extracted the embedded assets, so --tool, --features, and the extension remove/update completers offered nothing on a fresh install or after an upgrade. Built-in names now come from the binary in that case, and the installed-extension completer reads only the user root.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants